Repository navigation
feat: move Programs to its own service (EN-207) - #1290
Conversation
|
Important Review skippedToo many files! This PR contains 281 files, which is 181 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
|
corypride
left a comment
There was a problem hiding this comment.
@carddev81 I ran this locally with platform #21 and programs #1. Migrations, routing, transfer, deactivate, metrics and all 5 PDF reports work. With Programs down, nothing changes.
- A transfer that keeps failing still ends the enrollments. (Low) Programs is called before the target facility is checked (
user_handler.go:619). Transferring to a facility that doesn't exist returns 500 every time, but the enrollment is alreadyIncomplete: Transferred. Please validatetrans_facility_idbefore calling Programs. - Merge order. (Medium) The server won't start without
PROGRAMS_TOKEN(main.go:59-61), so this has to merge after platform #21's staging commit and the token sync. - Programs still writes
user_account_history. (Low) EN-207 rules that out, so it needs a sign-off or a ticket.
Will approve once finding 1 is fixed. Merge after programs #1 and platform #21.
Thanks for running it end to end.
|
corypride
left a comment
There was a problem hiding this comment.
@carddev81 Re-reviewed at 3fa9d60. Fix 1 is good: an unknown facility now gets a 400 from GetFacilityByID before Programs is called, TestTransferToUnknownFacility pins it, and CI is green. Your answers on 2 and 3 work for me.
One new thing: f6bf432 (“drop ticket numbers from code comments”) also uncommented PROGRAMS_DIR=../unlocked-programs in .env.example:68. Anyone who copies .env.example without that sibling checkout now gets make dev stopping with “there’s no checkout there”. Open Content and Provider Platforms leave theirs commented, so please comment it back out.
Approving, but make sure that line is commented out. Merge order as you listed it.
Pre-Submission PR Checklist
make devend to end still to run, see below)Description of the change
Core stops running the Programs module and calls the new unlocked-programs service instead.
Core asks Programs over HTTP with the contracts
programs.Client.PROGRAMS_URLdefaults tohttp://programs:8080.PROGRAMS_TOKEN, as it does for Open Content.Transfer, deactivate and bulk deactivate call Programs first, then run core's write.
ProgramsInTxis gone).Provider Platforms' live-provider calls (
coreapi.LiveProviders) go through a small adapter to Programs.Programs' routes and ingress:
rev_proxyin dev, send their paths to the service./api/users/{id}/programs,/attendance-trend,/attendance-exportand/program-completions→/api/program-users/{id}/.../api/facilities/{id}/instructors→/api/program-facilities/{id}/instructorsRemoved from core:
module.Depspieces only Programs used (HTTP,Features,Reports,ModuleDBs) andRunAs;GenerateReportPDFand the 5 Programs report templates. Core keeps Jasper and the usage report.make seedruns Programs'seed-devinstead);Tests:
TestProgramsCoreReadspasses with its goldens unchanged.testdata/programs_core_routes.Compose, make and README, like Open Content:
x-programsbuilds from GitHub (PROGRAMS_REF, defaultmain), or fromPROGRAMS_DIR.programs-migrate(afterserver-migrate),programson host port 8084, andprograms-worker.make migrate,migrate-freshandseedrun Programs'migrate,migrate-resetandseed-dev.Migration
00058's Down fixed: itscron_jobsinsert had no id, somigrate-freshfailed on main. Up is unchanged, so sites aren't affected.Contracts:
unlocked-contractsis pinned tov0.5.0.Related issues: https://unlockedlabs.atlassian.net/browse/EN-207
Screenshot(s)
No UI changes. The frontend only calls the renamed paths.
Additional context
Run:
tsc -b.Mutation checks failed the tests as they should:
Not verified yet: that a successful transfer sends Programs the right IDs. SQLite's single connection can't run that route. I'll check it in
make devbefore merge, along with the renamed pages.Watch:
PROGRAMS_TOKEN. It has to be incore-secretsbefore this deploys.Merge order (nothing enforces it):
v0.5.0.If any core features or components were removed: none for users. Programs runs as its own service.
Two notes: